Skip to content

Add/new listings importer#310

Open
eddiesshop wants to merge 42 commits intomasterfrom
add/new-listings-importer
Open

Add/new listings importer#310
eddiesshop wants to merge 42 commits intomasterfrom
add/new-listings-importer

Conversation

@eddiesshop
Copy link

@eddiesshop eddiesshop commented Sep 27, 2022

All Submissions:

Changes proposed in this Pull Request:

Closes # .

How to test the changes in this Pull Request:

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

How many Publishers was this used for so far? Do we have full example commands (CLI $ wp newspack-listings import ...) which were executed to import these

We’ve used it for the Rafu. I think Zak perhaps may have used the importer as well.
The command structure is like this: wp newspack-listings import /var/www/html/rafu-listings-cleaned.csv --mode=dry-run --pre-create-callback=/var/www/html/pre-create-callback-rafu.php,\\Rafu\\Rafu_Pre_Create_Callback. I think it could be improved if we change it to rely on apply_filters instead of using the PreCreateAbstractClass.

Have these templates been used on all the Publishers? How often, do you think, will we need to customize the templates for the Publishers?

These templates are supposed to be a “standard” listings template, much like how we have post blocks on the site. It’s supposed to make it a simpler experience/matter to import listings. It’s a decision I made to try to steer the listings development in a direction where it can be standardized in the future.
The code also allows for the use of a custom template if none of the “standard” ones are to be used

Looking at this template includes/templates/event/event.html it stands out a bit, but I don’t know how the wp:newspack-listings/event-dates bit works below. Could you share super briefly what the event template will output?

I like using the php function strtr (explanation here). It allows you to have placeholders in a string, which you can then replace with whatever you’d like. Sort of like sprintf except, you can name the string placeholder so that it’s easier to read and figure out what is expected to replace the placeholder.
So in the case of event-dates, the function would look like this:
strtr( $event_date_template, [ '{start_date}' => '2022-09-27' ] )
The above would generate a string like this:
<!-- wp:newspack-listings/event-dates {"startDate":"2022-09-27"} /-->

eddiesshop and others added 30 commits December 1, 2021 14:35
This factory will look at the file's extension and determine which class implementation to use.
The purpose of these classes is to allow for custom callback's which can be called before or after a record is created. This allows for a developer to perform data massaging or error reporting as an example.
- Using Newspack Listing Type pseudo enum class.
- Using Newspack Listing Import mode.
- Using Newspack Listing Type mapper.
- Fleshing out the import process for posts.
- Fleshing out the import process for images.
@eddiesshop eddiesshop requested a review from a team as a code owner September 27, 2022 13:14
@iuravic iuravic requested a review from a team September 27, 2022 14:04
@adekbadek
Copy link
Member

@eddiesshop – what's the status here? Is this still awaiting review, or maybe should be reverted to draft or closed?

@eddiesshop
Copy link
Author

Hey @adekbadek, while I do use this to import listings from time to time, it probably needs to be reworked a bit. There are some places where it would make more sense to use filters (for callbacks) instead of how it's currently set up. There's also a bit of complexity which could be simplified.

Ultimately though, I would ask @dkoo to see if he thinks this is even still necessary?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants